Skip to content

Add support for automatic database users for Postgres#25614

Merged
r0mant merged 1 commit intomasterfrom
roman/dbuser
May 18, 2023
Merged

Add support for automatic database users for Postgres#25614
r0mant merged 1 commit intomasterfrom
roman/dbuser

Conversation

@r0mant
Copy link
Copy Markdown
Collaborator

@r0mant r0mant commented May 4, 2023

Implements RFD 113.

A couple of minor optional things mentioned in the RFD are missing from the implementation (like auditing activate/deactivate queries). I'll add them later while doing more testing during review or as a follow up.

@r0mant r0mant requested review from greedy52 and smallinsky May 4, 2023 03:33
@github-actions github-actions Bot added database-access Database access related issues and PRs size/lg labels May 4, 2023
@github-actions github-actions Bot requested review from fspmarshall and hugoShaka May 4, 2023 03:33
@r0mant r0mant removed request for fspmarshall and hugoShaka May 4, 2023 03:34
Copy link
Copy Markdown
Contributor

@greedy52 greedy52 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First round. Will test it out next round.

Comment thread api/constants/constants.go Outdated
Comment thread lib/srv/db/common/role/role.go Outdated
Comment thread lib/srv/db/postgres/users.go
Comment thread lib/services/role.go
Comment thread lib/srv/db/postgres/users.go
Copy link
Copy Markdown
Contributor

@smallinsky smallinsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First pass.

Comment thread lib/config/fileconf.go
Comment thread lib/service/servicecfg/database.go
Comment thread lib/srv/db/common/interfaces.go Outdated
Comment thread lib/srv/db/common/autousers.go Outdated
Comment thread lib/srv/db/postgres/engine.go
@smallinsky smallinsky self-requested a review May 5, 2023 13:12
@r0mant
Copy link
Copy Markdown
Collaborator Author

r0mant commented May 10, 2023

@smallinsky @greedy52 I've addressed your feedback folks, mind taking another look?

@r0mant r0mant requested a review from greedy52 May 10, 2023 23:59
Comment thread lib/auth/auth.go Outdated
Copy link
Copy Markdown
Contributor

@greedy52 greedy52 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested with RDS today and this feature is awesome!

One small nit on a corner case, when admin user is not setup properly, message from tsh is not very obvious what's wrong sometimes:

$ tsh db connect --db-user STeve --db-name test postgres-rds
psql: error: connection to server at "localhost" (::1), port 58348 failed: Connection refused
	Is the server running on that host and accepting TCP/IP connections?
connection to server at "localhost" (127.0.0.1), port 58348 failed: FATAL:  password authentication failed for user "not-found"
ERROR: exit status 2

Comment thread tool/tsh/db.go Outdated
Comment on lines 872 to 880
Copy link
Copy Markdown
Contributor

@greedy52 greedy52 May 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a common problem where Teleport Connect code base may require extra changes.

Comment thread lib/services/role_test.go Outdated
Comment thread lib/srv/db/postgres/activate-user.sql Outdated
@r0mant r0mant requested a review from smallinsky May 16, 2023 00:45
@r0mant
Copy link
Copy Markdown
Collaborator Author

r0mant commented May 16, 2023

@smallinsky PTAL?

@r0mant r0mant force-pushed the roman/dbuser branch 8 times, most recently from ed4f7a6 to 9717c48 Compare May 18, 2023 18:27
@r0mant r0mant added this pull request to the merge queue May 18, 2023
Merged via the queue into master with commit 79b54d8 May 18, 2023
@r0mant r0mant deleted the roman/dbuser branch May 18, 2023 23:39
@public-teleport-github-review-bot
Copy link
Copy Markdown

@r0mant See the table below for backport results.

Branch Result
branch/v13 Failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

database-access Database access related issues and PRs size/lg

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants